[Experiment] Add freeform image cropper component #63335
Conversation
kevin940726
added
[Feature] Media
kevin940726
requested review from
talldan,
jasmussen,
richtabor,
ajlende and
ramonjd
|
Nice one, took it for a quick spin:
This is looking solid and promising. Small bug with zooming too much and it flips the image. This is very nascent, so hard to fully review until it's in a branch, implemented for the Image block, but a few general observations/thoughts:
Nice work. |
If the number of resize handles were reduced, there could be rotation handles on the corners, or some applications have a handle that extends out like this: |
|
This is feeling really nice to use so far! Another couple of questions, and possibly for @jasmussen (and once we integrate with the image block):
|
Most cropping tools that inspire this PR have their rotation slider below the image. For instance, here's Google Photos web: Kapture.2024-07-11.at.13.47.33.mp4The dimmed background is also needed in this case. I think we can also consider placing the slider at the bottom of the editing canvas? It can be toggled with a click in the block toolbar since it's an advanced usage.
I also considered this, but IMO this sort of UI only makes sense when you're rotating the cropping window itself, not the background image. Because this PR currently assumes that every cropped image should still be an unrotated rectangle, rotating the cropping window doesn't make sense here.
I think we should, at least for feature completion of accessibility. We can potentially add more keyboard controls too (Ctrl++/-?) in addition to the UI control. |
We need a fresh design for this. The following much older design could be a starting point, but it needs a fresh take that is more like the existing and with a smaller visual diff. For example, the modal may look fun at a quick glance, but let's not do this, it's not compatible with cropping tools that are likely to also exist in a future media library, and it also is a clunky amount of additional steps compared to a more lean and light-weight in-canvas cropping tool. I'm sharing it only because there may be some elements as far as aspect ratio that may be worth revisiting. Note specifically how the block toolbar itself becomes modal. Instead of having a single image block toolbar that grows wider with crop tools, this one proposes a drilldown effect for the block toolbar, affording better and clearer tools. Not sure how feasible, but it's worth resurfacing.
I like the slider in the toolbar, though I'm also aware that this is substantially more difficult to get right, than it looks at a glance. So also here it would seem useful to free up space in the block toolbar, however we can. Since it's already modal through the apply/cancel actions, perhaps that can be a way to take up more space in the block toolbar? |
That's a good point 😄 I think my main concern would be how this works in terms of inline editing when there are other blocks after the image, and also when there's a caption in that position. Something more minimal might be easier to wrangle. Having secondary UI controls in the inspector or toolbar is still always an option (and might be needed for accessibility). It can be something to iterate on though, it'll be interesting to see the designs. |
Kapture.2024-07-16.at.18.30.39.mp4I added the rotation (90deg clockwise and counter-clockwise) feature. The reason Currently, |
| }; | ||
| } | ||
|
|
||
| function imageCropperReducer( state: State, action: Action ) { |
There was a problem hiding this comment.
It'd be great to add some inline comments to the code within this reducer. Geometry code like this is notoriously hard to read.
I spotted a bug, if I resize twice from the bottom-right, the top-left of the resizable box jumps position incorrectly:
Kapture.2024-07-17.at.12.28.41.mp4
I found it a little difficult to understand the intention of the code in the RESIZE_WINDOW action, so some comments would help greatly.
There was a problem hiding this comment.
I found it a little difficult to understand the intention of the code in the
RESIZE_WINDOWaction, so some comments would help greatly.
Yeah, that's one of the things I need help with 😅. Half the time I don't even know what I'm doing 🤦. I'd need to re-organize my thoughts and refactor this in a way that makes it easier to write comments.
There was a problem hiding this comment.
The comment here might help - #63335 (comment).
There might be one or two things I got wrong because I'm mostly doing this as a mental exercise, but I think a good rule is to not modify the underlying image coordinates until the user hits 'apply'. Instead store the manipulations in a 'transform' object that describes how the image has been manipulated. It might be similar to what you're doing already, but I found it difficult to tell.
I think the important thing is to get the reducer working correctly and then have the UI work from that state. It might be challenging with some of the opinionated UI pieces like resizableBox, but a solid foundation for the maths will make everything work much more reliably.
| export type State = { | ||
| // The container/image's width. | ||
| width: number; | ||
| // The container/image's height. | ||
| height: number; | ||
| // The rotation angle between -45deg to 45deg. | ||
| angle: number; | ||
| // The number of 90-degree turns. | ||
| turns: 0 | 1 | 2 | 3; | ||
| // The zoom scale. | ||
| scale: number; | ||
| // Whether the image is flipped horizontally. | ||
| flipped: boolean; | ||
| // The offset position of the cropper window. | ||
| offset: Position; | ||
| // The position of center of the image. | ||
| position: Position; | ||
| // The size of the cropper window. | ||
| size: Size; | ||
| // Whether the cropper window is resizing. | ||
| isResizing: boolean; | ||
| // Whether the image is dragging/moving. | ||
| isDragging: boolean; | ||
| }; |
There was a problem hiding this comment.
A good aim could be to model the state like this:
State = {
image: {
// the image would always have a `top`/`left` of zero, because we apply `transform.translation` to move it. A rect is a web friendly storage format so might as well use it to keep things consistent.
rect: { top, right, bottom, left, x, y, width, height }
},
imageTransforms: {
rotate: number,
scale: number,
translate: Vector2<number>,
reflect: Vector2<bool>
},
// the resizable box
clippingRect: {
rect: { top, right, bottom, left, x, y, width, height }
},
// UI states
isResizing: bool,
isDragging: bool,
}I'm visualizing the image manipulation as two separate things.
Firstly, if you completely disregard the 'clipping rect', there are distinct changes the user can make to the image. They can rotate it (slider), zoom it (mouse wheel) and translate it (drag it). As the user makes those changes the deltas are added to the transform state. The image state doesn't change at all until the user clicks 'apply', at which point the transform state is reset to zero.
Keeping the transform as a state like this is pretty important as the scaling and rotation need to be applied before translation (moving) when transforming something geometrically around its center (assuming the origin is the center of the image, if not it can be translated to its center first before scaling, and then translated back again).
The clipping rect is a separate thing on top of that, but from what I can tell, it doesn't affect the underlying image until the user applies the crop, so I think a rect is the only state needed for it.
There might be more needed after that, for example I notice that the clipping rect and the image are often translated together (after resizing the clipping area, the image and clipping rect are recentered), but I think it'd be better to store that as a separate transform object (e.g. viewTransform) that's applied to the resizable box and the image (after the other transforms) so that they both move by the exact same amount.
This reflects pretty closely how video games work in having a series of 'transforms' (model, view, projection), though they store them in matrices:
https://gamedev.stackexchange.com/questions/178643/the-view-matrix-finally-explained
There was a problem hiding this comment.
I was also going to comment that turns might be more portable/easily understood as a rotate value, possibly in combination with the deg/radian?
There was a problem hiding this comment.
Do you mean a rotate and a degree value? I never like the naming of turns so I might switch to that 😅.
There was a problem hiding this comment.
Do you mean a rotate and a degree value?
Oh, the controls are separate, right? Then, yes 😄
| var( --wp-cropper-window-x ), | ||
| var( --wp-cropper-window-y ) | ||
| ); | ||
| box-shadow: 0 0 0 100vmax rgba( 0, 0, 0, 0.5 ); |
There was a problem hiding this comment.
This is a fantastic start. Awesome work @kevin940726
What do you think about adding some simple guide rulers? I'm using Google photos as inspiration 😄
Kapture.2024-07-22.at.15.39.57.mp4
Example
export const Resizable = styled( ResizableBox )`
transform: translate(
var( --wp-cropper-window-x ),
var( --wp-cropper-window-y )
);
box-shadow: 0 0 0 100vmax rgba( 0, 0, 0, 0.6 );
&:active {
box-shadow: 0 0 0 100vmax rgba( 0, 0, 0, 0.4 );
&::after,
&::before,
${ Draggable }::after, ${ Draggable }::before {
position: absolute;
padding: 0;
width: 1px;
height: 100%;
overflow: hidden;
left: 33.33%;
content: ' ';
background: rgba( 255, 255, 255, 0.33 );
z-index: 1000;
display: block;
}
&::before {
right: 33.33%;
left: auto;
}
${ Draggable }::before {
left: auto;
width: 100%;
height: 1px;
top: 33.33%;
}
${ Draggable }::after {
left: auto;
width: 100%;
height: 1px;
bottom: 33.33%;
top: auto;
}
}
`;Also, I know it's eye candy, but a subtle animation during the post-crop translate (not dragging) would look pleasing to my eye at least 🤗
There was a problem hiding this comment.
Nice! I think we can also show it on dragging and rotating too!.
There was a problem hiding this comment.
I pushed a commit that includes the rulers and the animations!
Kapture.2024-07-29.at.15.46.56.mp4
| ); | ||
| Default.args = { | ||
| src: 'https://upload.wikimedia.org/wikipedia/commons/thumb/3/34/Hydrochoeris_hydrochaeris_in_Brazil_in_Petr%C3%B3polis%2C_Rio_de_Janeiro%2C_Brazil_09.jpg/1200px-Hydrochoeris_hydrochaeris_in_Brazil_in_Petr%C3%B3polis%2C_Rio_de_Janeiro%2C_Brazil_09.jpg', | ||
| width: 250, |
There was a problem hiding this comment.
Do you think it would be helpful to provide an example of a crop container that resizes with the window/parent container?
There was a problem hiding this comment.
Yes, absolutely! It's on the TODO list but not supported yet. I think we can first implement the core features and then make it responsive. We also need to fix that it's not cropping the image in its original resolution 😅.
|
I added a section of features and TODOs in the PR description. Please let me know if any of them isn't clear! I'll try to add more too. |
|
Related to resizing the container, when we're using this in the block editor for the image block, the crop area should be the same size as the final image. One of the goals of core block editing is to make the editing mode look as close to the preview mode as possible. |
ajlende
force-pushed
the
add/freeform-image-cropper
branch
from
38d4ee5 to
797ec46
Compare
| dispatch( { | ||
| type: 'ROTATE_CLOCKWISE', | ||
| isCounterClockwise: true, | ||
| } ); |
There was a problem hiding this comment.
Nit: this API is a little confusing as one onehand it's ROTATE_CLOCKWISE and then on the other it's isCounterClockwise: true. Maybe the action type could be ROTATE_90 or something.
There was a problem hiding this comment.
Good point! I renamed it to ROTATE_90_DEG in 2fd0c69.
| angle: 0, | ||
| rotations: 0, |
There was a problem hiding this comment.
Out of interest, why does the angle needs to be stored separately to rotations? The two feel like the same thing to me.
There was a problem hiding this comment.
They are the same thing, but this matches the UI of the Google photos cropper that many of these interactions seemed to be based off of. One could deconstruct any angle into these two parts, but I haven't convinced myself which way would be better. Mostly thinking in terms of undo/redo.
There was a problem hiding this comment.
Yep! We need two separate values here so that we can distinguish between angle: 45, rotations: 0 and angle: -45, rotations: 1, which are both 45 degree but represent different states in the UI.
There was a problem hiding this comment.
Now that I've had some time to reflect on this, I think I would prefer the angle in radians as part of the transform, and in order to distinguish -45 from 45, the "tilt" can be saved as part of the UI state in degrees. This keeps the UI decoupled from the actual transform so it's easier to update the UI if needed.
|
I was playing around with more image croppers today, and the closest ones to what I had in mind for image block cropping were Adobe Express and Canva. Adobe ExpressAdobe Express has two resizable boxes, essentially; one for the crop region and one for the image. It showcases how rotation can be done in-canvas without a slider like Google. The part that doesn't work for us is that the positioning post crop will be different for us since we're working with an element that's part of the regular DOM flow and rules instead of a canvas where everything is absolutely positioned. Screen.Recording.2024-08-02.at.4.13.54.PM.movCanvaCanva "solves" the problem by not allowing the crop area to be changed while editing, so the size of the crop area in the page always matches the final result. Canva also adds a feature that keeps the crop are in bounds of the image that Adobe Express doesn't handle. Screen.Recording.2024-08-02.at.3.56.36.PM.movGoogle PhotosHowever, Google photos handles the reflow quite nicely by resizing the image on mouse up. Screen.Recording.2024-08-02.at.4.39.35.PM.movCombining themI think we should be able to combine the Adobe Express double box model with the Canva crop area bounds constraints and the Google Photos reflow mechanic to make something that will work in both the context of inline editing for the image block and as a fixed editor in the admin redesign. |
What do you have in mind for using the double box model for inline editing? If we reflow(scale) the image after resizing then the image box will grow bigger to the point that it might be difficult to fit the box into the screen 🤔. If the idea is to adopt the rotation button, then I think we can still do that using the Google Photos UX? Possibly something like a button rather than a slider below the cropper window? I think we can use some design feedback here too 😆. |
That's the kind of thing that concerns me too. Along with the possibility of having elements of the cropping tools overlapping other blocks. Engaging spotlight mode during cropping might be an option to keep focus on the image block. |
We do have a mode where the page is zoomed out, so we can scale the page to any size to make room for the "ghost" image outside of the page if needed. #63870 added an option for a fixed 50% scale with some other UI changes, but I don't see why we couldn't use that scaling to make more room for the contextual cropping. |
Wouldn't zoom-out mode also zoom out the image cropping area, thus making it smaller to interact with? Imagine a somewhat extreme example with a small cropping area.
If we scale up the cropping area to match the content size, the image background could be large enough that the rotation button is too far below to interact with. A solution would be moving the rotation button just below the cropping area, but I think that's essentially the same as the Google Photos example? 🤔 |
Yes, but I don't expect zoom out would be required for cropping. You'd probably only want to switch to zoom out mode when you're cropping things like full-width images.
That's a good point, especially since we don't really have an "infinite canvas" to work with like some of these design tools. My first reaction is that it wouldn't be a problem (from a usability perspective) to have both, but saying yes to everything can lead to a busy UI that's also hard to use. Would be good to get some design input on this. |
|
Size Change: +14.3 kB (+0.81%) Total Size: 1.78 MB
ℹ️ View Unchanged
|
|
In recent commits, you can now test the new image cropper in the Image block. Enable the gutenberg experiment "Redesigned image cropper" under "Gutenberg" -> "Experiments" -> "Redesigned image cropper".
After enabling the experiment, this redesigned cropper will be the default cropper of the image block. Insert an image block and test it in the editor. Kapture.2024-08-16.at.15.34.13.mp4I hope this makes testing this PR easier for designers and contributors. Needless to mention, there are still lots of improvements, and the state is not meant to be the final design. We'll add more features and fix bugs as we continue to develop the feature. Thanks for testing! 🙇 |
|
Thanks for working on this. This already feels vastly better than what exists. Nice work. I did encounter a glitch, I was using a portrait image, and resized it first, in doing this, toggling cropping on/off resulted in differing aspect ratios: One other aspect, when the images get very large, scrollbars get added. This is visible in your screenshots as well. Maybe this is fine? But it's worth testing this also on mobile, where I would worry that panning might pan the viewport rather than the image you're moving inside. Or worse yet, a bit of both which might get you in trouble. If we need to, can we simply crop off the edges outside your image frame? Right now they visually appear blurred, which is fancy, but in case it causes issues, simply not showing this might be fine. Separately, it might be time to make progress on toolbar improvements. At present, it looks like this: 
Notably Link and Crop buttons disappear from the toolbar, when you're cropping. This is reasonable, but also a bit random, since text overlay remains. Cropping is effectively a mode of the block toolbar, so we might as well lean into that. Could we do this? 
Doing this would also better fit the zoom/rotate/aspect ratio tools which are currently missing in this implementation. Like so: 
|
|
This works nicely so far, really great work here. There's a few little things I noticed from testing (you're probably aware some of them):
Kapture.2024-08-19.at.15.20.33.mp4
Kapture.2024-08-19.at.15.23.44.mp4
Kapture.2024-08-19.at.15.41.35.mp4 |
What?
The first draft of implementing a freeform image cropper component aiming to solve #22582.
Note
This PR is a WIP.
Please see the features and TODOs section below. We want to open a draft PR first to gather some very early feedback to ensure we are on the right track.
Why?
See #22582. The current image cropping tool lacks some advanced features we want to support. The library we used doesn't provide the features we need, and we can't find other third-party libraries that do. This component is an attempt to implement our own solution and potentially use it in the Media Library revamp.
How?
Lots of math 🧠.
Add a new
<ImageCropper>component in the@wordpress/componentspackage. Currently, it's not exported at all and only available in the storybook environment for testing. We'll export it as a private component next once we reach a point where it can be integrated into the block editor.The basic usage is as follow:
See the storybook for advanced usage.
Features and TODOs
Here's an unexhaustive list of supported features and TODOs with demo recordings.
Resizing
✅ Resize the cropping window
Kapture.2024-08-01.at.15.19.18.mp4
✅ Scale after resizing
Kapture.2024-08-01.at.15.19.18.mp4
✅ Resize after rotation
Kapture.2024-08-01.at.15.20.12.mp4
Resize max bounds
It should not be resized to be bigger than the image. Additionally, zooming out while resizing near the max bounds is nice-to-have. Here's a demo from the Google Photos iOS app:
RPReplay_Final1722504920.mov
Dragging
✅ Drag/pan the image
Kapture.2024-08-01.at.15.23.01.mp4
✅ Drag max bounds
Kapture.2024-08-01.at.15.23.34.mp4
✅ Drag after rotation
Kapture.2024-08-01.at.15.25.06.mp4
Rotation
✅ Rotate the image around the center of the cropping window
Kapture.2024-08-02.at.01.40.35.mp4
✅ Rotate 90 degrees (clockwise and counterclockwise)
Kapture.2024-08-01.at.15.25.06.mp4
✅ Scale after rotation
Kapture.2024-08-01.at.15.28.32.mp4
Rotation animation
The cropping window should also rotate with the image.
Kapture.2024-08-01.at.15.29.07.mp4
Flipping
✅ Flip the image horizontally
Flipping the image vertically can be achieved by rotating and flipping.
Kapture.2024-08-01.at.15.30.12.mp4
✅ Flip animation
Kapture.2024-08-01.at.15.30.12.mp4
Zooming
✅ Zoom in/out the image around the cursor's position by pinching
Kapture.2024-08-01.at.15.30.50.mp4
✅ Zoom after rotation
Kapture.2024-08-01.at.15.31.44.mp4
✅ Zoom max and min bounds
Currently, the maximum and minimum scale bounds are set to
1and10respectively.Zoom with a mouse or keyboard.
Resize after zooming out
Aspect ratio
Responsive
✅ Resize the container after resizing the window
It mostly works but has some bugs and lacks testing.
Accessibility
Styling
Integration
✅ Integrate into the Image block
Available under a gutenberg experiment "Redesigned image cropper".
Integrate into the Media Library (Phase 3: Collaboration > Media Library #55238)
Testing
Testing Instructions
For testing in the image block in the editor, enable the gutenberg experiment under "Gutenberg" -> "Experiments" -> "Redesigned image cropper".
Kapture.2024-08-16.at.15.34.13.mp4
For testing the component in the storybook:
npm run storybook:dev)http://localhost:50240/iframe.html?args=&id=components-imagecropper--inlineto view and play with the component.Testing Instructions for Keyboard
This component is currently not accessible to the keyboard, but it's on the plan.